Conversation
redefines set-content, breaks Start-PSES Fixes PowerShell/vscode-powershell#1331 I wish we didn't have to module qualify command names like this but we've been bitten several times now by overridden commands.
|
Wow that's annoying. But it seems necessary... |
|
Do we need to modify PowerShellEditorServices.psm1 as well? |
|
Yes. Good catch! I'll get that in a bit later tonight. |
|
It might be worth double checking the scripts that we use for remoting as well. psedit and such. |
I'm, for the moment, drawing the line at the *-Object commands. I really hate having to do this but I hate dealing with externally inflicted bugs even more.
TylerLeonhardt
left a comment
There was a problem hiding this comment.
LGTM - I feel like there should be a PSSA rule on redefining core cmdlets
That's a good idea. You should post that in the PSSA repo. |
|
This is relevant to this issue - PowerShell/PowerShell#7040 (comment) |
|
PowerShell/PSScriptAnalyzer#1023 did the thing! |
|
Whilst I know this is merged I would reccommend for defensive reasons across all of PSES you Module Qualify commands to ensure no one else breaks your functionality, perhaps in a similar way to how Pester does it |
can you elaborate on this? |
Pester defines a table of safe commands ahead of time because it has to be safe for mocking. That affects startup time and depends on being able to scope a variable. We could do it in a way that suits PSES better, but module-qualifying things seems pretty sufficient. You're asking for trouble if you duplicate a builtin module.
Precisely what this PR did, and we've done a bit more since. We also fully qualified native utils by path, since people like to install their own. There may still be some outliers left in things like C#-invoked PowerShell, but it's the kind of thing we're shaving down over time. |
redefines set-content, breaks Start-PSES
Fixes PowerShell/vscode-powershell#1331
I wish we didn't have to module qualify command names like this but
we've been bitten several times now by overridden commands.
One concern I have is testing this due to it needing to be signed.